Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Tie to the custom docker network for the devcontainers #1986

Merged
merged 8 commits into from
Nov 4, 2024
Merged

Tie to the custom docker network for the devcontainers #1986

merged 8 commits into from
Nov 4, 2024

Conversation

jimleroyer
Copy link
Member

@jimleroyer jimleroyer commented Nov 1, 2024 •
edited
Loading

Summary | Résumé

  • Attaching to the custom notify docker network (which should be created in the API project).
  • Refining a few building steps around the devcontainer build.
  • Deleting old docker-compose.yml file.
  • Installing a new docker-compose.yml file to get used by the devcontainer.

Test instructions | Instructions pour tester la modification

Rebuild the devcontainer and make sure you can start the application again.

Also, having the latest API with related changes, you can now perform the following while the API devcontainer is also up and running:

$ curl http://notify-api:6011 
{"current_time_utc": "2024-11-01 20:10:46.583150+00:00", "status": "ok", "commit_sha": "91efa1cb846a152574f6e6a537af43a8527a0114", "build_time": "2024-11-01:19:25:34", "db_version": "0460_new_service_columns"}

$ ping notify-api
PING notify-api (172.21.0.4) 56(84) bytes of data.
64 bytes from gc-notify-notify-api-1.notify-network (172.21.0.4): icmp_seq=1 ttl=64 time=0.127 ms
64 bytes from gc-notify-notify-api-1.notify-network (172.21.0.4): icmp_seq=2 ttl=64 time=0.065 ms
64 bytes from gc-notify-notify-api-1.notify-network (172.21.0.4): icmp_seq=3 ttl=64 time=0.064 ms

$ ping db        
PING db (172.21.0.3) 56(84) bytes of data.
64 bytes from gc-notify-db-1.notify-network (172.21.0.3): icmp_seq=1 ttl=64 time=0.081 ms
64 bytes from gc-notify-db-1.notify-network (172.21.0.3): icmp_seq=2 ttl=64 time=0.069 ms
64 bytes from gc-notify-db-1.notify-network (172.21.0.3): icmp_seq=3 ttl=64 time=0.062 ms

$ ping redis
PING redis (172.21.0.2) 56(84) bytes of data.
64 bytes from gc-notify-redis-1.notify-network (172.21.0.2): icmp_seq=1 ttl=64 time=0.080 ms
64 bytes from gc-notify-redis-1.notify-network (172.21.0.2): icmp_seq=2 ttl=64 time=0.060 ms
64 bytes from gc-notify-redis-1.notify-network (172.21.0.2): icmp_seq=3 ttl=64 time=0.061 ms

$ ping notify-admin
PING notify-admin (172.21.0.5) 56(84) bytes of data.
64 bytes from 1e032ac6fc8e (172.21.0.5): icmp_seq=1 ttl=64 time=0.021 ms
64 bytes from 1e032ac6fc8e (172.21.0.5): icmp_seq=2 ttl=64 time=0.039 ms
64 bytes from 1e032ac6fc8e (172.21.0.5): icmp_seq=3 ttl=64 time=0.036 ms

@jimleroyer jimleroyer self-assigned this Nov 1, 2024
@github-actions GitHub Actions
Copy link

github-actions bot commented Nov 1, 2024

🧪 Review environment

https://v3zrqml3zrsrikwhtgxzqycbo40xnjlr.lambda-url.ca-central-1.on.aws/

whabanks
whabanks approved these changes Nov 1, 2024
View reviewed changes
Copy link
Contributor

@whabanks whabanks left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, app runs and communicates with the API container just fine using host.docker.internal in the .env with Colima running.

@jimleroyer jimleroyer changed the title Fixing host.docker.internal Enable custom docker-compose notify network Nov 1, 2024
jimleroyer
jimleroyer commented Nov 1, 2024
View reviewed changes
.devcontainer/scripts/notify-dev-entrypoint.sh
chown -R vscode:vscode /workspaces/notification-admin
sudo -u vscode -i -- /usr/local/bin/installations.sh
# Install and setup dev environment
installations.sh
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was wondering how this script was called previously but could not find it. Does that look ok if I call it from here? cc @jzbahrai @whabanks @andrewleith

jimleroyer
jimleroyer commented Nov 1, 2024
View reviewed changes
.devcontainer/docker-compose.yml
@@ -0,0 +1,33 @@
version: '3'
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Having a docker-compose.yml file gives us more options to customize our VSCode devcontainers.

jimleroyer
jimleroyer commented Nov 1, 2024
View reviewed changes
.devcontainer/docker-compose.yml

networks:
notify-network:
external: true
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will attach to the custom network defined in the API project, where it is created.

@jimleroyer jimleroyer changed the title Enable custom docker-compose notify network Tie to the custom docker network for the devcontainers Nov 1, 2024
@amazingphilippe amazingphilippe mentioned this pull request Nov 4, 2024
Closed
@jimleroyer jimleroyer merged commit 26374b8 into main Nov 4, 2024
11 checks passed
@jimleroyer jimleroyer deleted the feat/internal-docker-host branch November 4, 2024 21:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@whabanks whabanks whabanks approved these changes

@andrewleith andrewleith Awaiting requested review from andrewleith

@amazingphilippe amazingphilippe Awaiting requested review from amazingphilippe

@sastels sastels Awaiting requested review from sastels

@jzbahrai jzbahrai Awaiting requested review from jzbahrai

@P0NDER0SA P0NDER0SA Awaiting requested review from P0NDER0SA

Assignees

@jimleroyer jimleroyer

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@jimleroyer @whabanks